maybe this should go into lifespan?

setup_frontend needs to happen at module load time otherwise uvicorn can't serve the routes. From what I understand, lifespan is for startup/shutdown tasks that are primarily async, not FastAPI route setup. That's why the comment is there right above 🙂 I'm happy to add to it to make this clearer if you have suggestions.

Both _get_attack_result_harm_category_condition and _get_attack_result_label_condition used to return an exists().where(...) condition, but now only assign it to a local variable (*_subquery) without returning it. That will make these helpers return None, breaking filtering logic that expects a SQLAlchemy condition. Return the constructed subquery (or revert to the inline return exists().where(...)).

Both _get_attack_result_harm_category_condition and _get_attack_result_label_condition used to return an exists().where(...) condition, but now only assign it to a local variable (*_subquery) without returning it. That will make these helpers return None, breaking filtering logic that expects a SQLAlchemy condition. Return the constructed subquery (or revert to the inline return exists().where(...)).

Returning full stack traces in the HTTP 500 detail leaks internal code structure and potentially sensitive runtime data to clients. Log the traceback server-side (optionally include request IDs), and return a generic error message to the client; if you want trace exposure for development, gate it behind a dev/debug flag.

Now that OpenAI targets use multiple inheritance elsewhere (e.g., RealtimeTarget(OpenAITarget, PromptChatTarget)), calling PromptTarget.__init__ directly bypasses cooperative initialization and can break MRO-based init chaining. Prefer super().__init__(...) in OpenAITarget.__init__ so all bases in the MRO can initialize correctly.

Now that OpenAI targets use multiple inheritance elsewhere (e.g., RealtimeTarget(OpenAITarget, PromptChatTarget)), calling PromptTarget.__init__ directly bypasses cooperative initialization and can break MRO-based init chaining. Prefer super().__init__(...) in OpenAITarget.__init__ so all bases in the MRO can initialize correctly.

Including activeConversationId in this effect dependency will refetch the full conversations list on every conversation selection, creating unnecessary API traffic and UI churn. Consider fetching on attackResultId changes (and after mutating actions like create/promote), but not on local selection changes.

Several new imports are placed inside test functions. For consistency and readability, prefer moving these imports to the module level (unless there’s a specific reason to defer import side effects).

Several new imports are placed inside test functions. For consistency and readability, prefer moving these imports to the module level (unless there’s a specific reason to defer import side effects).

Several new imports are placed inside test functions. For consistency and readability, prefer moving these imports to the module level (unless there’s a specific reason to defer import side effects).

Several new imports are placed inside test functions. For consistency and readability, prefer moving these imports to the module level (unless there’s a specific reason to defer import side effects).

The new inline insert logic introduces deferred/local imports (closing, SQLAlchemyError). This makes the method harder to reason about and diverges from the project’s import organization (imports at module top). Move these imports to the top of the file (or reuse already-imported equivalents) and keep the method focused on DB operations.

The E2E API tests don’t align with the backend contract: targets list uses limit (not count), and create uses type (not target_type). As written, the create test will likely always skip, reducing the value of the suite. Update these requests to match the API schema and avoid conditional skipping for expected/steady-state flows (use deterministic mocks or dedicated seeded endpoints if needed).

The base64 detection treats any non-data: and non-http string as base64 and wraps it into a data URI. Backend values can be file paths (e.g., /tmp/x.png, C:\\...) or non-http URLs (blob:, file:, azblob:), which will become invalid data URIs. Consider a stricter base64 check (e.g., regex/length validation) and/or explicit handling for known path/scheme prefixes before deciding to build a data URI.

These icon-only buttons rely on the title attribute for labeling. title is not consistently announced by screen readers and isn’t a reliable accessible name. Provide an explicit accessible label (e.g., aria-label) so the buttons have stable names for assistive tech and testing via role/name queries.

Exposing memory.engine.url.database in a public /version response can leak sensitive deployment details (e.g., server/database names, file paths). Consider returning only the backend type (or a redacted form), and/or only including database identifiers when running in a trusted/development mode.

created_at is modeled as Optional[str] here while other timestamps in the same API use datetime (e.g., AttackSummary.created_at/updated_at, CreateConversationResponse.created_at). Using datetime consistently improves schema clarity and avoids clients guessing formats; Pydantic will serialize it as ISO 8601 anyway.

This test suite mocks the module under test (./api) and re-implements the production logic inside the mock, which means it won’t catch regressions in frontend/src/services/api.ts. Prefer importing the real targetsApi/attacksApi and mocking only apiClient (e.g., via jest spies or an axios mock adapter) so the tests validate the actual implementation.

This loads the entire conversation just to check whether any prior turn exists. A cheaper approach is to query only for existence/first row (or count with a limit), which avoids pulling full histories into memory when a user accidentally reuses a conversation ID.